-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
sched/spin_lock: continue work to rename raw_spin* to spin_*_notrace #15862
base: master
Are you sure you want to change the base?
Conversation
Some improvements are made to the following commits: | commit f22b93b | Author: hujun5 <[email protected]> | Date: Fri Jan 31 07:01:07 2025 +0800 | | sched/spin_lock: rename raw_spin_lock to spin_lock_notrace | | Signed-off-by: hujun5 <[email protected]> Signed-off-by: chao an <[email protected]>
[Experimental Bot, please feedback here] No, this PR does not fully meet the NuttX requirements. While it provides a summary of the changes, it lacks crucial details. Here's a breakdown of the missing information:
In short: This PR needs significant improvement in terms of clarity, detail, and justification for the changes. It needs to stand alone and provide all the necessary context for a reviewer to understand and evaluate the changes effectively. Simply linking to another PR shifts the burden of understanding to the reviewer and makes it much harder to assess the validity and impact of this specific PR. |
Hi @anchao , I agree with these changes, but we have been discussing about code quality and how PRs should be provided (and reviewed). In that sense, please adhere to the guidelines to fill the PR's descriptions: the bot provide useful instructions to fill it here: #15862 (comment) The most important section is the Impact. Please provide how it impacts existing builds and how this impact could be mitigated. Also, provide a summary of why this change is happening (you can link previous discussions). In general, act as if you were a beginner on NuttX willing to help review a merge request: the MR must provide all the necessary info to it, starting by clearly pointing out the motivation for the change, and how it impacts the RTOS and a testing section. |
I think you should understand my original intention of submitting this patch, which is to solve the consistency problem in the current code #15767. If you don't want to approve it, just leave it pending here. I don't want 90% of my work to be about introducing what I'm doing. Open source for Nuttx is just my interest. If it becomes a burden to me, I'd rather not do it. |
Summary
sched/spin_lock: continue work to rename raw_spin* to spin_*_notrace
Some improvements are made to the following commits:
#15767
Signed-off-by: chao an [email protected]
Impact
N/A
Testing
ci-check